[BEEEP] Update bitwarden-ssh RustCrypto dependencies, fixing imports of various SSH keys #536

quexten · 2025-10-28T12:38:47Z

🎟️ Tracking

https://bitwarden.atlassian.net/browse/PM-27518

Fixes:
bitwarden/clients#17028
bitwarden/clients#14076

📔 Objective

Updates the bitwarden-ssh dependencies to rc releases from RustCrypto. This allows us to:

Drop our PKCS5 patch crate
Fix import of putty keys
Fix import of various other broken keys (linewidth issues and others)

It also replaces the RSA crate that has a vulnerability (timing attack) - though, it is unlikely that this actually affects import / was abusable.

RSA key generation test vectors are broken because the underlying RSA generation algorithm changed with rustcrypto's massive re-implementation of RSA, hence new test keys are provided.

Note: This pins the dependencies to be different from the workspace crates, as those are not yet ready to be updated. Eventually we will update those bit-by-bit too and then drop the dependency pinning.

⏰ Reminders before review

Contributor guidelines followed
All formatters and local linters executed and passed
Written new unit and / or integration tests where applicable
Protected functional changes with optionality (feature flags)
Used internationalization (i18n) for all UI strings
CI builds passed
Communicated to DevOps any deployment requirements
Updated any necessary documentation (Confluence, contributing docs) or informed the documentation
team

🦮 Reviewer guidelines

👍 (:+1:) or similar for great changes
📝 (:memo:) or ℹ️ (:information_source:) for notes or general info
❓ (:question:) for questions
🤔 (:thinking:) or 💭 (:thought_balloon:) for more open inquiry that's not quite a confirmed
issue and could potentially benefit from discussion
🎨 (:art:) for suggestions / improvements
❌ (:x:) or ⚠️ (:warning:) for more significant problems or concerns needing attention
🌱 (:seedling:) or ♻️ (:recycle:) for future improvements or indications of technical debt
⛏ (:pick:) for minor or nitpick changes

github-actions · 2025-10-28T12:47:54Z

Checkmarx One – Scan Summary & Details – 6119c753-9f33-4ba7-bdc1-33e27427ed53

Great job! No new security vulnerabilities introduced in this pull request

claude · 2025-10-29T18:42:13Z

Claude finished @quexten's task —— View job

Code Review Complete

The dependency updates to RustCrypto RC versions are well-executed and successfully address the stated objectives. Test coverage for new functionality is strong.

Findings

Finding 1 ⚠️ Dependency version pinning diverges from workspace without documentation

The PR pins exact versions (=0.11.0-rc.7, =3.0.0-pre.1) in bitwarden-ssh/Cargo.toml while other crates use workspace ranges. While the PR description mentions this is temporary, there's no inline documentation or FIXME comment indicating these pins should be updated later. (Inline comment posted)

Context

crates/bitwarden-ssh/Cargo.toml:

ed25519-dalek = { version = "=3.0.0-pre.1", features = ["alloc", "pkcs8"] }
pkcs8 = { version = "=0.11.0-rc.7", features = ["encryption"] }

vs workspace in Cargo.toml:

ed25519-dalek = { version = ">=2.1.1, <=2.2.0" }

bitwarden-crypto uses workspace version ed25519-dalek = { workspace = true } (line 40).

Finding 2 💭 Generic error conversion loses diagnostic information

In import.rs:42-45, specific pkcs8::Error variants are mapped to generic SshKeyImportError::Parsing, losing potentially useful diagnostic information for debugging. This pattern exists in the current code and wasn't introduced by this PR, but it's worth considering for future improvements whether intermediate error context should be preserved.

Finding 3 🎨 Eliminated manual RSA conversion workaround

The removal of convert_rsa_keypair() in export.rs demonstrates that the upstream RustCrypto fix has been properly integrated. The new TryInto trait implementation is cleaner and eliminates the workaround code.

Finding 4 🎨 Comprehensive test coverage for bug fixes

New regression tests effectively validate fixes:

import_ed25519_key_regression_17028 - Tests issue #17028
import_key_ed25519_putty / import_key_rsa_openssh_putty - Tests putty key support
Updated test expectations from "should fail" to "should succeed"

Summary of Changes

Dependencies Updated:

ssh-key: 0.6.x → 0.7.0-rc.3
ed25519: 2.x → 3.0.0-rc.1
ed25519-dalek: workspace 2.x → pinned 3.0.0-pre.1
pkcs8: 0.10.x → 0.11.0-rc.7
rsa: 0.9.x → 0.10.0-rc.9
rand: 0.8.x → 0.9.2
Removed: Custom pkcs5 patch from workspace

Code Changes:

export.rs: Removed manual RSA conversion workaround; now uses TryInto trait (lines 28-38)
generator.rs: Updated RNG trait from CryptoRngCore to CryptoRng + rand::rng() (lines 2-4, 30, 36, 49)
import.rs: Fixed type parameters for PEM label matching (lines 25-26)
WASM build: Added getrandom_backend="wasm_js" cfg flag + feature gate

Test Updates:

RSA test vectors regenerated (3072/4096 keys)
New test cases for putty-exported keys (3 new test resource files)
Regression test for issue #17028

Action Items

✅ Inline comment added for dependency pinning
Consider whether error context should be preserved in import.rs error mappings for future improvements
Verify CI passes with new dependency versions
Plan workspace-wide dependency update when other crates are ready for RustCrypto RC versions

Security & Best Practices

✅ No logging of sensitive key material
✅ No unsafe blocks introduced
✅ Error handling maintains security (password errors distinct from parsing errors)
✅ RSA timing attack vulnerability mitigated by upgrading rsa crate
✅ Backward compatibility maintained (OpenSSH format unchanged)
✅ Test coverage includes security-sensitive cases (wrong password, malformed keys)

codecov · 2025-10-30T19:52:22Z

Codecov Report

❌ Patch coverage is 96.55172% with 1 line in your changes missing coverage. Please review.
✅ Project coverage is 78.66%. Comparing base (8ef7951) to head (47325ab).
⚠️ Report is 1 commits behind head on main.

Files with missing lines	Patch %	Lines
crates/bitwarden-ssh/src/generator.rs	66.66%	1 Missing ⚠️

Additional details and impacted files

@@            Coverage Diff             @@
##             main     #536      +/-   ##
==========================================
- Coverage   78.70%   78.66%   -0.04%     
==========================================
  Files         296      296              
  Lines       30500    30504       +4     
==========================================
- Hits        24005    23997       -8     
- Misses       6495     6507      +12

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:

❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

claude · 2025-11-07T09:00:33Z

crates/bitwarden-ssh/Cargo.toml

+getrandom = { version = "=0.3.3" }
+pem-rfc7468 = "1.0.0-rc.3"
+pkcs8 = { version = "=0.11.0-rc.7", features = ["encryption"] }
+rand = "0.9.2"


⚠️ Finding 1: Consider adding a comment here indicating these pinned versions should be updated when the workspace is ready:

Suggested change

rand = "0.9.2"

# TODO: Update to workspace versions when other crates are ready for RustCrypto RC releases

rsa = "0.10.0-rc.9"

This helps track that the version pinning is temporary and should be revisited.

quexten added 2 commits October 28, 2025 13:37

Update ssh dependencies and fix imports of various SSH keys

601fafb

Cargo fmt

56960c3

quexten and others added 5 commits October 28, 2025 13:48

Add test pubkeys

34e3346

Cargo fmt

ecbf697

Fix wasm build

ed8b8e9

Add new lines

0e6a4ce

Merge branch 'main' into km/beeep/ssh-import-fix

68702a1

quexten marked this pull request as ready for review October 29, 2025 18:41

quexten requested a review from a team as a code owner October 29, 2025 18:41

quexten requested a review from addisonbeck October 29, 2025 18:41

quexten changed the title ~~[BEEEP] Update ssh dependencies and fix imports of various SSH keys~~ [BEEEP] Update bitwarden-ssh RustCrypto dependencies, fixing imports of various SSH keys Oct 29, 2025

Merge branch 'main' into km/beeep/ssh-import-fix

47325ab

addisonbeck approved these changes Oct 31, 2025

View reviewed changes

claude bot reviewed Nov 7, 2025

View reviewed changes

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

[BEEEP] Update bitwarden-ssh RustCrypto dependencies, fixing imports of various SSH keys #536

[BEEEP] Update bitwarden-ssh RustCrypto dependencies, fixing imports of various SSH keys #536

Uh oh!

quexten commented Oct 28, 2025 •

edited

Loading

Uh oh!

github-actions bot commented Oct 28, 2025 •

edited

Loading

Uh oh!

claude bot commented Oct 29, 2025 •

edited

Loading

Uh oh!

codecov bot commented Oct 30, 2025 •

edited

Loading

Uh oh!

claude bot Nov 7, 2025

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

	rand = "0.9.2"
	# TODO: Update to workspace versions when other crates are ready for RustCrypto RC releases
	rsa = "0.10.0-rc.9"

[BEEEP] Update bitwarden-ssh RustCrypto dependencies, fixing imports of various SSH keys #536

Are you sure you want to change the base?

[BEEEP] Update bitwarden-ssh RustCrypto dependencies, fixing imports of various SSH keys #536

Uh oh!

Conversation

quexten commented Oct 28, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

🎟️ Tracking

📔 Objective

⏰ Reminders before review

🦮 Reviewer guidelines

Uh oh!

github-actions bot commented Oct 28, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Great job! No new security vulnerabilities introduced in this pull request

Uh oh!

claude bot commented Oct 29, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Code Review Complete

Findings

Summary of Changes

Action Items

Security & Best Practices

Uh oh!

codecov bot commented Oct 30, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Codecov Report

Uh oh!

claude bot Nov 7, 2025

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

quexten commented Oct 28, 2025 •

edited

Loading

github-actions bot commented Oct 28, 2025 •

edited

Loading

claude bot commented Oct 29, 2025 •

edited

Loading

codecov bot commented Oct 30, 2025 •

edited

Loading